Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #3701 +/- ##
==========================================
- Coverage 89.73% 89.72% -0.02%
==========================================
Files 904 904
Lines 105871 105946 +75
==========================================
+ Hits 95007 95057 +50
- Misses 10864 10889 +25
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
jsiirola
left a comment
There was a problem hiding this comment.
A couple minor questions that don't need to hold up the PR and one significant one that probably does (it look like the ASL solution loader is not loading all import suffixes).
| for k, v in solution_loader.get_duals(solution_id=solution_id).items(): | ||
| dual_suffix[k] = v |
There was a problem hiding this comment.
Some questions:
- Should the Suffix be cleared before loading?
- Could this just be:
| for k, v in solution_loader.get_duals(solution_id=solution_id).items(): | |
| dual_suffix[k] = v | |
| dual_suffix.update(solution_loader.get_duals(solution_id=solution_id)) |
| vars_to_load: list | ||
| A list of the variables whose solution value should be retrieved. If vars_to_load | ||
| is None, then the values for all variables will be retrieved. | ||
| solution_id: Optional[Any] |
There was a problem hiding this comment.
Instead of Optional[Type], I believe the current "preferred Python" annotation is Type | None. In this case, since Any includes None, I think you could just do Any (although Any | None is a little more explicit).
| def load_import_suffixes(self, solution_id=None): | ||
| load_import_suffixes(self._pyomo_model, self, solution_id=solution_id) |
There was a problem hiding this comment.
This only loads duals and rc ... the ASL supports arbitrary inpirt suffixes, which I think need to be supported here.
There was a problem hiding this comment.
Can you give me an example of what ASLSolFileData would look like if it had other suffixes. I see that it has attributes like var_suffixes. I assume the keys are the names of the suffixes, and the values map the variable indices to the suffix values? Any suggestions on how to test this?
There was a problem hiding this comment.
Yes: Suffix data comes back from the ASL flagged for "variables", "constraints", "objectives", and the "problem". We blindly parse that into the corresponding attributes (var_suffixes, etc).
- For
var,con, andobj, the dict maps suffix name to a dict that maps the 0-based integer index of the relevant variable / constraint / objective in the corresponding list in theNLWriterInforeturned by the writer tot he suffix value. - For the problem, since there is only one problem, the
problem_suffixesdict just maps suffix name to value.
Summary/Motivation:
This PR updates the solution loader in pyomo/contrib/solver based on recent design discussions.
Changes proposed in this PR:
get_number_of_solutionsmethodget_solution_idsmethodload_solution_methodload_import_suffixesmethodsolution_idas an argument to methodsLegal Acknowledgement
By contributing to this software project, I have read the contribution guide and agree to the following terms and conditions for my contribution: